Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect cursor hide/show to stderr #456

Merged
merged 1 commit into from
Jun 30, 2021
Merged

Conversation

wagoodman
Copy link
Contributor

#448 pulled in an external lib to perform hiding/showing the cursor. However, this needs to be done relative to the right output stream (in our case, stderr), otherwise syft | less will continue to show the cursor.

This PR restores this functionality.

@wagoodman wagoodman added the bug Something isn't working label Jun 30, 2021
@wagoodman wagoodman requested a review from a team June 30, 2021 16:49
@wagoodman wagoodman self-assigned this Jun 30, 2021
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman wagoodman force-pushed the redirect-etui-cursor-to-stderr branch from c3eaf9d to c87f349 Compare June 30, 2021 16:50
@github-actions
Copy link

Benchmark Test Results

Benchmark results from the latest changes vs base branch
name                                                   old time/op    new time/op    delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2           895µs ± 2%     763µs ± 1%  -14.80%  (p=0.008 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2        1.24ms ± 5%    1.03ms ± 1%  -16.97%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2     457µs ± 3%     384µs ± 1%  -15.94%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                 448µs ± 4%     365µs ± 1%  -18.49%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                  485µs ± 2%     386µs ± 1%  -20.39%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                  6.24ms ± 9%    5.22ms ± 1%  -16.39%  (p=0.008 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                  696µs ± 2%     580µs ± 0%  -16.69%  (p=0.008 n=5+5)
ImagePackageCatalogers/go-cataloger-2                     257µs ± 4%     206µs ± 1%  -19.91%  (p=0.008 n=5+5)
ImagePackageCatalogers/rust-cataloger-2                   372µs ± 4%     312µs ± 1%  -16.16%  (p=0.008 n=5+5)

name                                                   old alloc/op   new alloc/op   delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2          97.6kB ± 0%    97.5kB ± 0%     ~     (p=0.151 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2         579kB ± 0%     579kB ± 0%   -0.03%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2     112kB ± 0%     112kB ± 0%   -0.07%  (p=0.032 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                 116kB ± 0%     115kB ± 0%   -0.19%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                  134kB ± 0%     134kB ± 0%   +0.00%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                  1.80MB ± 0%    1.79MB ± 0%     ~     (p=0.151 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                 1.14MB ± 0%    1.14MB ± 0%     ~     (p=0.071 n=5+5)
ImagePackageCatalogers/go-cataloger-2                    53.9kB ± 0%    53.9kB ± 0%   -0.10%  (p=0.032 n=5+5)
ImagePackageCatalogers/rust-cataloger-2                  88.9kB ± 0%    88.9kB ± 0%   +0.00%  (p=0.008 n=5+5)

name                                                   old allocs/op  new allocs/op  delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2           1.96k ± 0%     1.96k ± 0%     ~     (all equal)
ImagePackageCatalogers/python-package-cataloger-2         5.89k ± 0%     5.89k ± 0%     ~     (all equal)
ImagePackageCatalogers/javascript-package-cataloger-2     1.93k ± 0%     1.93k ± 0%     ~     (all equal)
ImagePackageCatalogers/dpkgdb-cataloger-2                 2.37k ± 0%     2.37k ± 0%     ~     (all equal)
ImagePackageCatalogers/rpmdb-cataloger-2                  3.19k ± 0%     3.19k ± 0%     ~     (all equal)
ImagePackageCatalogers/java-cataloger-2                   22.3k ± 0%     22.3k ± 0%     ~     (p=0.333 n=4+5)
ImagePackageCatalogers/apkdb-cataloger-2                  1.85k ± 0%     1.85k ± 0%     ~     (all equal)
ImagePackageCatalogers/go-cataloger-2                     1.44k ± 0%     1.44k ± 0%     ~     (all equal)
ImagePackageCatalogers/rust-cataloger-2                   2.75k ± 0%     2.75k ± 0%     ~     (all equal)

Copy link
Contributor

@dakaneye dakaneye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be the first to admit my ANSI skills are lacking, but from what I can tell in the code changes, it looks like things were already logging to stderr, and had some questions re: the actual hide and show cursor methods (sorry if those questions are things I should know already)

The other question I have is if there's any testing required (but you've mentioned before that we haven't typically unit tested the ETUI stuff in the past)

return nil
}

func hideCursor(output io.Writer) {
fmt.Fprint(output, "\x1b[?25l")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we get a method comment to add some context here? I have some general questions that might be implicit in the method name, but want to make sure...

  • what does the \x1b[?25l code correspond to?
  • why does printing it to the specified output stream hide the cursor?

These questions also apply to the showCursor method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry @dakaneye , just as I merged your message popped up (I didn't refresh). I'll come back after the next couple of meetings and provide further context here 👍

@wagoodman wagoodman merged commit fee35dd into main Jun 30, 2021
@wagoodman wagoodman deleted the redirect-etui-cursor-to-stderr branch June 30, 2021 17:10
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants